[MP] Gracefully handle cache confirmation of deleted nodes #90027
Conversation
It's possible that after sending a cached node reference (e.g. RPC or static MultiplayerSynchronizer) the reference node is removed from tree before the remote peer(s) can confirm the referenced path. To better detect that case, and avoid spamming errors when it happens, this commit modifies the multiplayer API caching protocol, to send the received ID instead of the Node path when sending the confirmation packet. **This is a breaking change** because it makes the runtime multiplayer protocol incompatible with previous versions of Godot.
I don't remember where, but didn't we decide that we don't expect different Godot versions to be MP protocol compatible anyway? So a networking breaking change is one that makes networking code written for a previous version not work anymore; that a new Godot version can't connect to an older version without issues is just expected? I don't really know many MP games where you can connect to a server of a different game/engine version without issue. The code changes here LGTM, going to test this with some projects to make sure nothing breaks. |
Yes, we mentioned that multiple times, but I couldn't find a clear mention of it in the docs (something we might want to specify in the high-level multiplayer tutorial).
There's godotengine/godot-proposals#3533 which keeps popping to mind when I do these breaking changes. I think we could provide an "auth_callback" that performs the version check as an addon at least. The problem is that to cover all cases (or, well, most of the cases), it would probably need to be implemented at the mid-level abstraction layer (MultiplayerPeer). The reason for the "breaks compat" label is more to ensure a mention in the release blog post, to avoid people getting surprised by it. |
|
Might this pr also fix this error on server when client kills the app without properly disconnecting the peer? (4.2.2) |
No, that appears to be a different problem, |
|
Thanks! |
It's possible that after sending a cached node reference (e.g. RPC or static MultiplayerSynchronizer) the reference node is removed from tree before the remote peer(s) can confirm the referenced path.
To better detect that case, and avoid spamming errors when it happens, this commit modifies the multiplayer API caching protocol, to send the received ID instead of the Node path when sending the confirmation packet.
This is a breaking change because it makes the runtime multiplayer protocol incompatible with previous versions of Godot.
Fixes #85883
Fixes #86909